[OpenTelemetry] Drop non-finite values from histogram aggregations by martincostello · Pull Request #7759 · open-telemetry/opentelemetry-dotnet

martincostello · 2026-09-15T15:20:02Z

Changes

Non-finite values are no longer included in histograms to avoid permanently poisoning cumulative histogram aggregations.

From the metrics specification:

Implementations SHOULD NOT incorporate non-normal values (i.e., +Inf, -Inf, and NaNs) into the sum, min, and max fields, because these values do not map into a valid bucket.

This was found from a scan of the repository using Claude Code - it seems correct to me given the above, unless there's some nuance I've missed.

Merge requirement checklist

CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
Unit tests added/updated
Appropriate CHANGELOG.md files updated for non-trivial changes
~~Changes in public API reviewed (if applicable)~~

Non-finite values are no longer included in histograms to avoid permanently poisoning cumulative histogram aggregations.

codecov · 2026-09-15T15:39:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.10%. Comparing base (1e88560) to head (3a74d3c).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7759      +/-   ##
==========================================
+ Coverage   93.09%   93.10%   +0.01%     
==========================================
  Files         397      399       +2     
  Lines       23335    23604     +269     
  Branches     4359     4406      +47     
==========================================
+ Hits        21724    21977     +253     
- Misses       1103     1108       +5     
- Partials      508      519      +11

Flag	Coverage Δ
unittests-Project-Experimental	`93.22% <100.00%> (+0.01%)`	⬆️
unittests-Project-Stable	`93.19% <100.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/OpenTelemetry/Metrics/AggregatorStore.cs	`91.25% <100.00%> (+0.34%)`	⬆️
...c/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs	`91.80% <100.00%> (+0.71%)`	⬆️

... and 8 files with indirect coverage changes

Add PR number.

Copilot

🟡 Changes recommended

Filtering occurs after tagged metric-point allocation, allowing invalid measurements to consume cardinality and affect valid streams.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates histogram aggregation to drop non-finite measurements before aggregation.

Changes:

Filters NaN and infinities from histogram updates.
Adds regression coverage and updates Prometheus snapshots.
Documents the behavior change.

File summaries

File	Description
`src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs`	Filters non-finite histogram values.
`src/OpenTelemetry/CHANGELOG.md`	Documents the behavior change.
`test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs`	Adds cumulative histogram coverage.
`test/OpenTelemetry.Tests/Metrics/MetricTestData.cs`	Updates min/max expectations.
`test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramNaN.verified.txt`	Updates NaN output snapshot.
`test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramInfinities.verified.txt`	Updates infinity output snapshot.

Review details

Files reviewed: 6/6 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

opentelemetry-pr-dashboard · 2026-09-17T06:23:17Z

Pull request dashboard status

Waiting on the author · refreshed 2026-10-28 13:24 UTC

Move out of draft to request review.

Status above doesn't look right?

Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

- Do not consume cardinality slots for histograms when updating if only non-finite values. - Add missing test coverage.

Copilot

🟡 Changes recommended

The new test uses a banned constructor and removes coverage for streams containing only non-finite measurements.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 8/8 changed files
Comments generated: 2
Review effort level: Balanced

Add missing test coverage.

Copilot

🟢 Approval recommended

The implementation consistently filters invalid measurements before allocation and aggregation, with comprehensive regression coverage.

Review details

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Balanced

github-actions · 2026-09-26T03:25:43Z

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

Kielek · 2026-10-07T15:43:25Z

I have created open-telemetry/opentelemetry-specification#5346

If you want to merge this PR shortly, I would suggest to narrow changes to the exponential histograms only. We can extend this, when we have clarification from spec team.

martincostello · 2026-10-07T15:53:54Z

If you want to merge this PR shortly

No rush on merge, I just wanted some opinions on whether the changes were right with regards to the spec or not.

Can wait on the outcome of open-telemetry/opentelemetry-specification#5346.

martincostello · 2026-10-08T09:11:43Z

Moving to draft until open-telemetry/opentelemetry-specification#5346 is resolved.

Move entry.

Move entry back to Unreleased.

Copilot

🟡 Changes recommended

The exponential histogram comments incorrectly call negative values undefined and should instead describe them as rejected by this SDK path without changing the existing guard.

1 open issue

Clarify negatives are rejected by this SDK path, not undefined · New

2 resolved since last review

This rejects the value only after AggregatorStore has resolved or created a tagged MetricPoint.…
Replacing these cases removes the only all-non-finite histogram input…

_{Review effort: Balanced}

Update comment to be more accurate.

Copilot

🔵 Needs a closer look

Maintainer confirmation is needed because the cited non-finite-value guidance covers exponential histograms, while dropping explicit-histogram counts and bucket observations would broaden SDK policy.

No open issues 🎉

1 resolved since last review

Clarify negatives are rejected by this SDK path, not undefined

_{Review effort: Balanced}

[OpenTelemetry] Drop non-finite histogram values

fb90431

Non-finite values are no longer included in histograms to avoid permanently poisoning cumulative histogram aggregations.

github-actions Bot added pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package labels Sep 15, 2026

martincostello commented Sep 15, 2026

View reviewed changes

Comment thread src/OpenTelemetry/CHANGELOG.md Resolved

[OpenTelemetry] Update CHANGELOG

a3ef8ad

Add PR number.

martincostello requested a balanced review from Copilot September 15, 2026 16:36

Copilot started reviewing on behalf of martincostello September 15, 2026 16:48

View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Comment thread src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs Resolved

Comment thread test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs Resolved

Merge branch 'main' into drop-nonfinite-from-views

47c5453

opentelemetry-pr-dashboard Bot mentioned this pull request Sep 17, 2026

Pull Request Dashboard #7539

Open

[OpenTelemetry] Address feedback

7256ade

- Do not consume cardinality slots for histograms when updating if only non-finite values. - Add missing test coverage.

martincostello requested a balanced review from Copilot September 17, 2026 10:45

Copilot started reviewing on behalf of martincostello September 17, 2026 10:46

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs Resolved

Comment thread test/OpenTelemetry.Tests/Metrics/MetricTestData.cs Resolved

[OpenTelemetry] Address feedback

c8a54e9

Add missing test coverage.

martincostello requested a balanced review from Copilot September 17, 2026 11:36

Copilot started reviewing on behalf of martincostello September 17, 2026 11:37

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

martincostello marked this pull request as ready for review September 17, 2026 11:54

martincostello requested a review from a team as a code owner September 17, 2026 11:54

Merge branch 'main' into drop-nonfinite-from-views

09de2b6

github-actions Bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Sep 26, 2026

martincostello added keep-open Prevents issues and pull requests being closed as stale and removed Stale Issues and pull requests which have been flagged for closing due to inactivity labels Sep 26, 2026

Merge branch 'main' into drop-nonfinite-from-views

95b5cd3

martincostello requested a review from matt-hensley October 7, 2026 11:09

Merge branch 'main' into drop-nonfinite-from-views

6a129f5

Merge branch 'main' into drop-nonfinite-from-views

fe7b759

martincostello marked this pull request as draft October 8, 2026 09:11

martincostello added needs-spec-change Issues which require the OpenTelemetry Specification to clarify or define behavior spec-compliance Issues related to compliance with the OpenTelemetry Specification labels Oct 8, 2026

martincostello added 13 commits October 20, 2026 13:54

Merge branch 'main' into drop-nonfinite-from-views

7baf43c

[OpenTelemetry] Update CHANGELOG

013c29e

Move entry.

Merge branch 'main' into drop-nonfinite-from-views

967485a

Merge branch 'main' into drop-nonfinite-from-views

c8178a7

Merge branch 'main' into drop-nonfinite-from-views

5c54bc9

Merge branch 'main' into drop-nonfinite-from-views

be996ce

Merge branch 'main' into drop-nonfinite-from-views

61704c6

Merge branch 'main' into drop-nonfinite-from-views

e5d2e6d

Merge branch 'main' into drop-nonfinite-from-views

3519e6d

Merge branch 'main' into drop-nonfinite-from-views

460fde0

[OpenTelemetry] Update CHANGELOG

8a02a6f

Move entry back to Unreleased.

Merge branch 'main' into drop-nonfinite-from-views

e8cd0c4

Merge branch 'main' into drop-nonfinite-from-views

6372ee2

martincostello requested a balanced review from Copilot November 1, 2026 15:39

Copilot started reviewing on behalf of martincostello November 1, 2026 15:40

View session

Copilot AI reviewed Nov 1, 2026

View reviewed changes

Comment thread src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs Resolved

[OpenTelemetry] Address feedback.

8ff8ddf

Update comment to be more accurate.

martincostello requested a balanced review from Copilot November 1, 2026 18:44

Copilot started reviewing on behalf of martincostello November 1, 2026 18:44

View session

Copilot AI reviewed Nov 1, 2026

View reviewed changes

martincostello added 3 commits November 2, 2026 10:08

Merge branch 'main' into drop-nonfinite-from-views

c6671f5

Merge branch 'main' into drop-nonfinite-from-views

5d18da4

Merge branch 'main' into drop-nonfinite-from-views

3a74d3c

Conversation

martincostello commented Sep 15, 2026

Changes

Merge requirement checklist

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

opentelemetry-pr-dashboard Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request dashboard status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 26, 2026

Uh oh!

Kielek commented Oct 7, 2026

Uh oh!

martincostello commented Oct 7, 2026

Uh oh!

martincostello commented Oct 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Sep 15, 2026 •

edited

Loading

opentelemetry-pr-dashboard Bot commented Sep 17, 2026 •

edited

Loading